H2: separate feed status evidence from producer result - #45
Conversation
Co-Authored-By: Codex <noreply@openai.com>
🤖 Codex PR Review🚫 Merge blocked: 2 serious issue(s) found in high-risk files 🚫 Blocking IssuesThese issues must be fixed before this PR can be merged: 1. 🟠 [HIGH] Logic in
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3b502a582
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if statuses and not any(item.ok for item in statuses): | ||
| if not feed_records: | ||
| raise ValueError("feed_config_empty") | ||
| if any(record["state"] == "failed" for record in feed_records): |
There was a problem hiding this comment.
When at least one feed is quarantined and none failed, this condition is false, so fetch_rss_sources exits 0 after writing a status with eligible_for_live_publication=false. I checked .github/workflows/rss_source_pipeline.yml: the publish step only depends on the command succeeding and copies data/output/rss_source_pipeline/source_fetch_status.json into data/live, and repo search shows no workflow check for eligible_for_live_publication; a zero-entry feed can therefore still publish live source_items/source_events that the new status marks non-eligible. Either fail this path for quarantines in the scheduled publisher or add a workflow gate before copying live outputs.
Useful? React with 👍 / 👎.
| item_count=0, | ||
| error=f"{type(exc).__name__}: {exc}", | ||
| ) | ||
| code = getattr(exc, "code", "fetch_failed") |
There was a problem hiding this comment.
When fetch_url or parse_feed_snapshot raises FeedXmlError("feed_xml_oversize") or FeedXmlError("feed_schema_invalid") under --continue-on-feed-error, FeedXmlError has no code attribute, so this falls back to fetch_failed and the canonical status loses the sanitized root cause. Since those XML errors are already producer-boundary codes and tests distinguish them at the exception boundary, use the exception argument or give FeedXmlError a code attribute; otherwise operators cannot tell bad/oversized payloads from network fetch failures in the new status file.
Useful? React with 👍 / 👎.
| if data["configured_feed_count"] != len(feeds) or data["feed_count"] != len(feeds): | ||
| _fail("status_counter_invalid") |
There was a problem hiding this comment.
For a parsed status payload with feeds=[] and all counters set to zero, this count check passes and the later complete = failed == 0 and quarantined == 0 logic accepts eligible_for_live_publication=true. That contradicts the new producer invariant that empty feed configuration is invalid (build_status([]) raises feed_config_empty), so any consumer that uses parse_status_bytes as the contract validator can accept an impossible empty snapshot as publication-ready.
Useful? React with 👍 / 👎.
| if status_output: | ||
| write_fetch_status(status_output, statuses, item_count=0) | ||
| write_fetch_status(status_output, feed_records) | ||
| raise RuntimeError("all configured RSS/Atom feeds failed") |
There was a problem hiding this comment.
When a partial feed failure occurs in the inspected RSS Source Pipeline, this raise happens after writing source_fetch_status.json but before the workflow reaches the Upload RSS source artifact step, which has the default success-only behavior. The new hard-fail path therefore discards the canonical evidence file from the Actions run for the accepted+failed case that used to complete and upload artifacts; make the upload/status preservation step run on failure or defer the raise until after evidence is externally retained.
Useful? React with 👍 / 👎.
| if any(child.tag not in allowed and child.tag not in _RSS_FOREIGN_METADATA for child in channel): | ||
| raise FeedXmlError("feed_schema_invalid") |
There was a problem hiding this comment.
For RSS 2.0 feeds that put namespaced syndication metadata directly under <channel> (for example sy:updatePeriod or sy:updateFrequency), this rejects the entire feed before item extraction even though the previous parser ignored unknown channel children and still parsed <item> rows. Since feed configs are operator-provided and these extension elements are common on otherwise valid RSS feeds, a single source with such metadata becomes a failed feed and now aborts the run under the new any-failed behavior.
Useful? React with 👍 / 👎.
1 participant
Scope
Fresh H2 from latest
origin/main@294e30b8eed36f070299b1dc01d34955fb262918; PR #44 remains permanently frozen evidence and was not reused.This producer-side slice separates canonical status evidence from the producer result:
No workflow, publish, weekly, QAR, permission, or automation changes are included.
Validation